Update openshift/snc cloud-init commands #633
Conversation
| - chmod 0644 /opt/crc/pass_developer | ||
| - echo "{{ .PublicIP }}" > /opt/crc/eip | ||
| - chmod 0644 /opt/crc/eip | ||
| - /usr/local/bin/crc-aws-fetch-secrets.sh "{{ .SSMPullSecretName }}" "{{ .SSMKubeAdminPasswordName }}" "{{ .SSMDeveloperPasswordName }}" |
There was a problem hiding this comment.
🤔 are you gonna add helpers for each major cloud provider?
I understand this helper would improve the fetch secrets time...but it kind of feel like a black magic to know how to use this linkage for having them...previously it was pretty explicit how we set and get those values.
Not against just saying seems a bit back magic xD
There was a problem hiding this comment.
there are different things that are important to have:
- ability the write full-fledged scripts (so more than this cloud-init command list)
- encapsulation (don't split the logic over too many multiple parties)
- confidentiality for the secret values (don't pass them via command line arguments)
so here I move the "black box" away from mapt, and let the SNC define it (here, just fyi).
another option could be to have this file inside mapt, and let mapt (via cloud-init) provide the file and call it.
I'm fine with any option.
I like the idea that for the supported environment(s), SNC provides the script which handles the secrets, to avoid any leak, rather than a "third party" component. But it can be further discussed, depending on the cloud env and deployers that CRC wants to support
There was a problem hiding this comment.
Sure, it is just we know this helper is there and we can make use of it, it does not mean others need to use it. Also I am thinking it will not block any new provider as I can do it with the cloud specific service as I did here and then eventually you can move that to a script inside SNC (but that would be more maintenance on SNC side).
The other caveat is..if I merge this PR old bundles will not work :( so let me hold this one...or split it at least until we have a clear decission.... Istio is onboarding this on Openshift-CI and they are using 4.19.12 ...so until we have someting to tell them to use I would prefer to hold
WDYT?
There was a problem hiding this comment.
sure, we'll sync on Monday to see how to get this merged without breaking any user workflow
| bootcmd: | ||
| # Resize the partition (4 = /dev/nvme0n1p4 typically) | ||
| - growpart /dev/nvme0n1 4 | ||
| # Resize the XFS filesystem on /sysroot |
There was a problem hiding this comment.
But it was resized some how... AFAIK the disk was over the original 32 / 42 snc image disk size no?
There was a problem hiding this comment.
this part (the disk resize), we'll have to rethink it, this part was broken, and another script was broken
I think we'll try to let cloud-init handle it all
There was a problem hiding this comment.
Ok that is pretty important...as with 34G and not resize to requested disk size... users can not use it..actually we already hit that issue and this is why we introduced this here
|
Also I know at this point the current version of mapt is supporting the latest snc |
hum, that may be something to rediscuss, as it will break some compatibility between old and new mapt/snc version |
2 participants
This PR makes the openshift/snc loader use the new helper script provided by the openshift/snc image to prepare the CRC secrets.
Must be merged after this PR: crc-org/snc#1168